[ObsUX] Add Missing transaction warning if there are Transactions or Spans with a parent.id that doesn't exist in the trace #171196

jennypavlova · 2023-11-14T13:34:19Z

Summary

Add a missing transaction warning if there are Transactions or Spans with a parent.id that doesn't exist in the trace.

Testing

Use the trace_with_orphan_items.ts scenario: node scripts/synthtrace --clean trace_with_orphan_items.ts
In APM -> Traces there are 2 traces:
Check the traces:
- Incomplete trace (with warning):
- Complete trace (no warning):
Unit test in waterfall_helpers.test.ts

apmmachine · 2023-11-14T13:34:37Z

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

/oblt-deploy : Deploy a Kibana instance using the Observability test environments.
/oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

…ransactions-are-missing

elasticmachine · 2023-11-16T12:32:49Z

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

cauemarcondes · 2023-11-20T12:29:59Z

...details/waterfall_with_summary/waterfall_container/waterfall/missing_transaction_warning.tsx

+  return (
+    <EuiToolTip
+      position="left"
+      delay="regular"


regular is the default value, so no need to specify this prop.

Thanks, removed ✅

cauemarcondes · 2023-11-20T12:31:22Z

...rfall_with_summary/waterfall_container/waterfall/waterfall_helpers/waterfall_helpers.test.ts

+      },
+    } as WaterfallTransaction;
+
+    it('should return false if there is not orphan items', () => {


Suggested change

it('should return false if there is not orphan items', () => {

it('should return false if there are no orphan items', () => {

Forgot that after I changed item with items, fixed 👍

cauemarcondes · 2023-11-20T12:31:35Z

...rfall_with_summary/waterfall_container/waterfall/waterfall_helpers/waterfall_helpers.test.ts

+      expect(getHasOrphanTraceItems(traceItems)).toBe(false);
+    });
+
+    it('should return true if there is orphan items', () => {


Suggested change

it('should return true if there is orphan items', () => {

it('should return true if there are orphan items', () => {

cauemarcondes · 2023-11-20T12:39:55Z

.../waterfall_with_summary/waterfall_container/waterfall/waterfall_helpers/waterfall_helpers.ts

+  const waterfallItemsIds = traceDocs.map((doc) =>
+    doc.processor.event === 'span'
+      ? (doc?.span as WaterfallSpan['span']).id
+      : doc?.transaction?.id
+  );
+
+  return traceDocs.some(
+    (item) => item.parent?.id && !waterfallItemsIds.includes(item.parent?.id)
+  );


The waterfall itself is already very slow, WDYT of this change to speed this function up? With this you change it from O(n) to O(1).

Suggested change

const waterfallItemsIds = traceDocs.map((doc) =>

doc.processor.event === 'span'

? (doc?.span as WaterfallSpan['span']).id

: doc?.transaction?.id

);

return traceDocs.some(

(item) => item.parent?.id && !waterfallItemsIds.includes(item.parent?.id)

);

const waterfallItemsIds = new Set(traceDocs.map((doc) =>

doc.processor.event === 'span'

? (doc?.span as WaterfallSpan['span']).id

: doc?.transaction?.id

));

return traceDocs.some(

(item) => item.parent?.id && !waterfallItemsIds.has(item.parent.id)

);

FYI: I did not test it.

Nice idea, thank you! I changed that and tested it, looks good 👍

elasticmachine · 2023-11-20T12:41:09Z

Pinging @elastic/apm-ui (Team:APM)

…ransactions-are-missing

cauemarcondes

LGTM

…ransactions-are-missing

kpatticha

🥳 🥳 Thanks for fixing it.

kibana-ci · 2023-11-20T16:20:56Z

💚 Build Succeeded

Buildkite Build
Commit: 71ee05c

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`apm`	1520	1521	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`apm`	3.7MB	3.7MB	+1.1KB

History

💔 Build #176940 failed 39b0e7b
💛 Build #176471 was flaky 5c027d6
💔 Build #176298 failed 7096e45640d25076f900d6953f0dbed94b4822a9
💔 Build #176193 failed 47e4197
💔 Build #175720 failed a970d5a

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @jennypavlova

sorenlouv · 2023-11-21T22:28:28Z

...blic/components/app/transaction_details/waterfall_with_summary/waterfall_container/index.tsx

+          </EuiFlexItem>
+          {hasOrphanTraceItems ? (
+            <EuiFlexItem grow={false}>
+              <MissingTransactionWarning />


nit: I suggest naming this OrphanTraceItemsWarning to align with hasOrphanTraceItems

sorenlouv · 2023-11-21T22:38:50Z

...details/waterfall_with_summary/waterfall_container/waterfall/missing_transaction_warning.tsx

+        'xpack.apm.transactionDetails.agentMissingTransactionMessage',
+        {
+          defaultMessage:
+            'This trace contains spans from missing transactions. As a result these spans are not displayed in the timeline.',


The error message is not entirely correct afaict. Example: a (parent) span could be missing, meaning that any child spans (or transactions) will not be displayed. So it is not just about missing transactions.
Also, we should suggest work-arounds for the user if possible.

Suggestion for error message:

This trace is incomplete and {{count}} items could not be displayed in the timeline. This could be a temporary problem caused by ingest delay, or a permanent problem caused by some events being dropped.

Thank you, I remember I took the message from the issue description, and now that you mentioned that it makes sense. One question regarding the {{count}} - we want to show the missing items in the waterfall based on the items returned and the items shown in the waterfall, right? I guess here we just calculate it, I don't see the missing items count existing somewhere (similar to the check we have here but instead of returning a boolean it should return the missing items count)

I added a draft PR for those changes so we can better discuss there

One question regarding the {{count}} - we want to show the missing items in the waterfall based on the items returned and the items shown in the waterfall, right? I guess here we just calculate it, I don't see the missing items count existing somewhere (similar to the check we have here but instead of returning a boolean it should return the missing items count)

Yes, I was thinking it would be beneficial to at least show the number of (orphan) items that cannot be placed because a parent is missing.

If it's difficult to calculate this, let's just skip it for now.

#173196) Closes #173134 ## Summary This PR is a follow-up to #171196 and changes the missing trace items tooltip message. <img width="1630" alt="image" src="https://github.com/elastic/kibana/assets/14139027/ae13547c-f2fb-4a19-8062-2774ea782111"> ## Testing Same as in #171196 (comment) but the message should be changed to the one in the screenshot here

Add Missing transaction warning if there are spans without parent id

a970d5a

jennypavlova added the Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team label Nov 14, 2023

jennypavlova self-assigned this Nov 14, 2023

jennypavlova mentioned this pull request Nov 14, 2023

[APM] Trace timeline: Show message when transactions are missing #25117

Closed

jennypavlova added 2 commits November 15, 2023 15:35

Check for orphan items in a trace and show warning

47e4197

Use correct trace docs

5281ca3

jennypavlova changed the title ~~[ObsUX] Add Missing transaction warning if there are spans without parent id~~ [ObsUX] Add Missing transaction warning if there are Transactions or Spans with a parent.id that doesn't exist in the trace Nov 15, 2023

Add scenario for trace with orphan items

45317fa

jennypavlova force-pushed the 25117-apm-trace-timeline-show-message-when-transactions-are-missing branch from 7096e45 to 45317fa Compare November 16, 2023 10:38

Merge branch 'main' into 25117-apm-trace-timeline-show-message-when-t…

5c027d6

…ransactions-are-missing

jennypavlova added release_note:skip Skip the PR/issue when compiling release notes backport:skip This commit does not require backporting labels Nov 16, 2023

jennypavlova marked this pull request as ready for review November 16, 2023 12:32

jennypavlova requested a review from a team as a code owner November 16, 2023 12:32

cauemarcondes reviewed Nov 20, 2023

View reviewed changes

botelastic bot added the Team:APM All issues that need APM UI Team support label Nov 20, 2023

CR changes

f5f3973

jennypavlova requested a review from a team as a code owner November 20, 2023 14:00

Merge branch 'main' into 25117-apm-trace-timeline-show-message-when-t…

39b0e7b

…ransactions-are-missing

jennypavlova requested a review from cauemarcondes November 20, 2023 14:03

cauemarcondes approved these changes Nov 20, 2023

View reviewed changes

jennypavlova added 2 commits November 20, 2023 15:55

Fix scenario type issue using apmEsClient

09e9a3e

Merge branch 'main' into 25117-apm-trace-timeline-show-message-when-t…

71ee05c

…ransactions-are-missing

kpatticha approved these changes Nov 20, 2023

View reviewed changes

jennypavlova merged commit 3dbe1e4 into elastic:main Nov 20, 2023

jennypavlova deleted the 25117-apm-trace-timeline-show-message-when-transactions-are-missing branch November 20, 2023 16:33

kibanamachine added the v8.12.0 label Nov 20, 2023

sorenlouv reviewed Dec 12, 2023

View reviewed changes

jennypavlova mentioned this pull request Dec 12, 2023

[ObsUx] Trace timeline: Change the missing trace items warning message #173196

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ObsUX] Add Missing transaction warning if there are Transactions or Spans with a parent.id that doesn't exist in the trace #171196

[ObsUX] Add Missing transaction warning if there are Transactions or Spans with a parent.id that doesn't exist in the trace #171196

jennypavlova commented Nov 14, 2023 •

edited

Loading

apmmachine commented Nov 14, 2023

elasticmachine commented Nov 16, 2023

cauemarcondes Nov 20, 2023

jennypavlova Nov 20, 2023

cauemarcondes Nov 20, 2023

jennypavlova Nov 20, 2023

cauemarcondes Nov 20, 2023

cauemarcondes Nov 20, 2023 •

edited

Loading

jennypavlova Nov 20, 2023

elasticmachine commented Nov 20, 2023

cauemarcondes left a comment

kpatticha left a comment

kibana-ci commented Nov 20, 2023

sorenlouv Nov 21, 2023 •

edited

Loading

sorenlouv Nov 21, 2023 •

edited

Loading

jennypavlova Dec 12, 2023

jennypavlova Dec 12, 2023

sorenlouv Dec 13, 2023 •

edited

Loading

	it('should return false if there is not orphan items', () => {
	it('should return false if there are no orphan items', () => {

	it('should return true if there is orphan items', () => {
	it('should return true if there are orphan items', () => {

[ObsUX] Add Missing transaction warning if there are Transactions or Spans with a parent.id that doesn't exist in the trace #171196

[ObsUX] Add Missing transaction warning if there are Transactions or Spans with a parent.id that doesn't exist in the trace #171196

Conversation

jennypavlova commented Nov 14, 2023 • edited Loading

Summary

Testing

apmmachine commented Nov 14, 2023

🤖 GitHub comments

elasticmachine commented Nov 16, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cauemarcondes Nov 20, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Nov 20, 2023

cauemarcondes left a comment

Choose a reason for hiding this comment

kpatticha left a comment

Choose a reason for hiding this comment

kibana-ci commented Nov 20, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

sorenlouv Nov 21, 2023 • edited Loading

Choose a reason for hiding this comment

sorenlouv Nov 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sorenlouv Dec 13, 2023 • edited Loading

Choose a reason for hiding this comment

jennypavlova commented Nov 14, 2023 •

edited

Loading

cauemarcondes Nov 20, 2023 •

edited

Loading

sorenlouv Nov 21, 2023 •

edited

Loading

sorenlouv Nov 21, 2023 •

edited

Loading

sorenlouv Dec 13, 2023 •

edited

Loading